Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show the context menu verb only when shift key is down #9358

Closed
wants to merge 2 commits into from

Conversation

mooons
Copy link

@mooons mooons commented Mar 3, 2021

Summary of the Pull Request

References

Related: #8105

PR Checklist

  • Closes The explorer context menu should only appear on shift+click #6113
  • CLA signed. If not, go over here and sign the CLA
  • Tests added/ passed (Invoke-OpenConsoleTests Total=329, Passed=329)
  • Documentation updated. If checked, please file a pull request on our docs repo and link it here: #xxx
  • Schema updated.
  • I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Haven't found a proper way to test ShellExtension so I went creative: (much appreciated if anybody can point me to the proper way to test this :p)

  • Installed Windows Terminal (stable) from Microsoft Store
  • Built x64 Release WindowsTerminalShellExt.dll
  • Copied built WindowsTerminalShellExt.dll under C:\Program Files\WindowsApps\
  • (Optional) Copied WindowsTerminal.exe under C:\Program Files\WindowsApps\ for verb icon
  • Under HKCR\PackagedCom\Package\Microsoft.WindowsTerminal_1.6.10571.0_x64__8wekyb3d8bbwe\Class\{9F156763-7844-4DC4-B2B1-901F640F5155}\, changed DllPath to ..\WindowsTerminalShellExt.dll
  • Manually verified the change in explorer -- works as expected.

In a folder, shift up;
1 in dir shift up

In a folder, shift down:
2 in dir shift down

On a folder, shift up:
3 on dir shift up

On a folder, shift down:
4 on dir shift down

@ghost ghost added Area-ShellExtension For issues related to the explorer right-click context menu Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. labels Mar 3, 2021
Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, this is exactly how I'd imagine this would be implemented. I'm gonna block this PR for a bit to give the team a chance to discuss in our next team sync - probably next Monday.

IMO, I definitely want there to be a setting to configure this, since people are totally split on whether they want this to appear on Click or Shift+Click. However, we don't have a really good way of configuring settings for the context menu quite yet. Setting up the context menu to be able to read the settings isn't something I'd ask of a community member, but we haven't really committed writing that code to a particular milestone quite yet. So I'm afraid that this PR might sit open for a while while we figure that out.

The code's totally fine, I just wanted to give you a heads up. Thanks!

@ghost ghost added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Mar 3, 2021
@mooons
Copy link
Author

mooons commented Mar 3, 2021

Thanks @zadjii-msft .

I agree the configurable context menu part can be a bit tricky. From my limited understanding of the code base atm, ShellExtension doesn't seem to like the headers in TerminalSettingsModel right now otherwise it should be possible to allow ShellExtension to read global settings? On DLL load (WindowsTerminalShellExt.dll), read config once. Whenever context menu related config changes, save json then reload the DLL.

Or, a IPropertySetStorage or something similar could act as a bridge in-between?

Just some random thoughts. :D

@ghost ghost removed the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Mar 3, 2021
@zadjii-msft
Copy link
Member

Hey so sorry to leave this one on read for so long. Apparently, I never came back and replied to this thread last year when we had this discussion 😕

IIRC, we were cool with the idea, but concurred that it needed to be something that needed a setting to toggle this behavior. As it stands, there's probably no line-of-sight on actually providing settings for the context menu entry at this time 😢 We've got a lot of cool ideas, just not any time to implement them. At this time last year, I think we were still hoping to get to doing the settings for the context menu a little faster, but alas, changing resourcing and priorities has tossed those plans to the wind.

I also don't want to ask you to try and hook these things up, either. Theoretically, you could just add a project reference to Terminal.Settings.Model, and include the settings model header in pch.h. That wouldn't be too hard, I don't think, since I think the WindowsShellExt project is already using cppwinrt.

My only big worry here is what the perf impact of loading the settings when the context menu is invoked. We'd have to also make sure to obey fOkToBeSlow.... It's a lot to ask of an external contributor. I'm certainly willing to help if you'd like to take a crack at it again.

Again, apologies for leaving this on read so long. That's on me.

@zadjii-msft zadjii-msft added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label May 5, 2022
@ghost ghost added the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label May 12, 2022
@ghost
Copy link

ghost commented May 12, 2022

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

@ghost ghost closed this May 19, 2022
@Phroneris
Copy link

So, until this feature is approved, are there any workarounds for users?
How can I edit Registry to do the same?

The known method #7008 (comment) removes the menu completely, not just when Shift isn't pressed.

@microsoft-github-policy-service microsoft-github-policy-service bot removed the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Mar 17, 2023
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-ShellExtension For issues related to the explorer right-click context menu Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The explorer context menu should only appear on shift+click
3 participants